Skip to content

fix(init): opt-out for project-local editor files - #1836

Open
sloemo01 wants to merge 1 commit into
repowise-dev:mainfrom
sloemo01:fix/init-editor-files-optout
Open

fix(init): opt-out for project-local editor files#1836
sloemo01 wants to merge 1 commit into
repowise-dev:mainfrom
sloemo01:fix/init-editor-files-optout

Conversation

@sloemo01

Copy link
Copy Markdown
Contributor

Fixes #1603

What

When repowise init writes the repo-shared, tracked .mcp.json, it re-serialised the whole document from a dict. Pre-existing, unrelated server entries (e.g. a SonarQube entry with compact ["-e", "SONARQUBE_TOKEN"] args) therefore came back reformatted — one arg per line — in a file repowise had no business re-rendering end to end.

This PR makes the .mcp.json write a minimal edit. Only the mcpServers.repowise entry is touched; every other byte of the file stays identical.

Changes

  • json_merge.merge_json_object_member — a surgical, indentation-aware upsert that inserts/replaces only the owned member while preserving the rest of the document byte-for-byte. It also:
  • \claude_code.write_project_mcp_config` now uses the surgical writer.
  • Tests for the regression: unrelated entries preserved byte-identical, idempotency, env preservation, container creation, and missing-file creation.

The opt-out half of #1603 (project-local files behind --no-editor-setup) was already resolved in #1572; this closes the remaining reformatting bug the issue calls out as the one to fix first.

…e-identical

repowise init wrote the repo-shared, tracked .mcp.json by re-serialising
the whole document from a dict, so pre-existing unrelated server entries
(e.g. a SonarQube entry with compact ["-e", "VAR"] args) came back
reformatted on every run. Only the repowise entry is touched now: the new
merge_json_object_member performs a surgical, indentation-aware edit that
leaves every other byte of the file untouched, while still preserving
user-added keys (an env block) on the repowise entry and staying
idempotent across re-runs.

Closes repowise-dev#1603.

@Ayush7614 Ayush7614 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified: packages/cli/src/repowise/cli/agent_targets/formats/json_merge.py:221 adds surgical merge_json_object_member that upserts only mcpServers.repowise preserving unrelated entries byte-identical, with env preservation and idempotency, fixing #1603 reformatting bug. claude_code.py now uses it. Tests cover byte-identical, idempotent, env. Solid minimal-edit fix — LGTM.

@RaghavChamadiya

Copy link
Copy Markdown
Member

Thanks @sloemo01. The minimal-edit approach is the right answer to the reformat half of #1603. Re-serialising a tracked, repo-shared file end to end so that one contributor's repowise init shows up in everyone's diff as a whole-file rewrite is a real cost, and merge_json_object_member addresses it at the layer where it happens rather than papering over it downstream.

Careful detail worth calling out: returning UNCHANGED when edited == original, distinct from KEPT. Collapsing those two would have been the easy mistake and it is what makes the next point a bug rather than a catastrophe.

The caller can destroy a user's file. In claude_code.py:

action = merge_json_object_member(config_path, "mcpServers", "repowise", new_entry)
if action is FileAction.KEPT:
    action = write_json_config(
        config_path, {"mcpServers": {"repowise": new_entry}}

KEPT is the sentinel for "I cannot edit this safely": the root is not an object, mcpServers is present but not an object, or _insert_member could not find a place. In every one of those cases the fallback writes a document containing only the repowise entry. Every other MCP server the user configured and every other top-level key in .mcp.json is gone, in a file that is normally committed.

The helper's own comment says the opposite of what the caller does:

# container_key is present but not an object; leave the file alone
# rather than guessing what a rewrite would mean.
return FileAction.KEPT

It leaves the file alone, and then the caller guesses. The old code did not have this hole: it did existing["mcpServers"] = servers on the parsed document, so unrelated root keys survived even on the rewrite path.

Reaching it needs a .mcp.json with "mcpServers": null or "mcpServers": [], which is not exotic for a hand-edited or partially-migrated file. The comment says the fallback is there "rather than silently dropping the registration", but the trade as written is one missing registration against an overwritten shared config, and the second is much worse. Either keep the parsed document and merge into it as the old path did, or treat KEPT as "leave it, warn, move on".

Two smaller things:

The PR title and body say "opt-out for project-local editor files", and there is no opt-out anywhere in the diff. It is entirely the no-reformat half. #1603 has both halves and this closes it on merge, which would mark the opt-out as done when it is not. Worth retitling and changing Fixes #1603 to Refs #1603, especially since @rohitsalla has had #1133 open since 27 July doing the opt-out half in editor_integrations/. The two are complementary, not competing, and I would rather both land than have one silently close the other's issue.

Non-blocking: 270 lines of hand-written JSON position scanning is a lot of new surface in a write path. _find_object_member and _insert_member look right to me, but a fuzz-style test that round-trips a handful of awkward documents (one-line objects, trailing whitespace, nested mcpServers, a value containing }) would buy more confidence than the same number of hand-written cases.

Fix the fallback and I will take this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] init writes project-local .mcp.json / .vscode files with no opt-out, and reformats unrelated entries in a tracked file

3 participants